-
-
Notifications
You must be signed in to change notification settings - Fork 6.4k
feat: Introduce Downloads Archive page #7794
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds a new simplified downloads page that’s fully server-rendered (no JS required) and provides major‐version download summaries, with expandable minor versions under <details>
. It also centralizes download utilities and updates the download URL API to use an options object.
- Introduce
simplified.mdx
andDownloadSimpleLayout
for the new page - Update
getNodeDownloadUrl
signature and its callers to use an options object - Add
DownloadsTable
,Details
, andWithSimplifiedDownload
components
Reviewed Changes
Copilot reviewed 23 out of 23 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
util/downloadUtils/index.tsx | Exported DownloadDropdownItem type |
util/downloadUtils/constants.json | Added compatibility ranges for platforms |
util/tests/getNodeDownloadUrl.test.mjs | Updated tests for new getNodeDownloadUrl signature |
types/layouts.ts | Added 'download-simple' layout type |
pages/en/download/simplified.mdx | New MDX page with simplified download UI |
next.mdx.use.mjs | Registered new MDX components (WithSimplifiedDownload , etc.) |
next.dynamic.constants.mjs | Configured dynamic routing and exclusions for new page |
layouts/DownloadSimple.tsx | New layout wrapping simplified download content |
components/withSimplifiedDownload.tsx | HOC to provide download data and sidebar items |
components/withProgressionSidebar.tsx | Refactored to accept both navKey and explicit groups |
components/withMetaBar.tsx | Made items injectable for meta‐bar |
components/withMarkdownContent.tsx | New HOC to load MDX content at runtime |
components/withLayout.tsx | Added DownloadSimpleLayout to layout mapping |
components/MDX/Details | New <Details> component for collapsible sections |
components/Downloads/Release/ReleaseCodeBox.tsx | Updated “no-script” link to point at new simplified page |
components/Downloads/Release/PrebuiltDownloadButtons.tsx | Switched getNodeDownloadUrl calls to object syntax |
components/Downloads/DownloadsTable | New table for listing artifacts |
components/Downloads/DownloadLink.tsx | Updated to new getNodeDownloadUrl API |
components/Downloads/DownloadButton | Updated to new getNodeDownloadUrl API |
Comments suppressed due to low confidence (1)
apps/site/components/Downloads/DownloadsTable/index.tsx:1
- Consider adding unit tests for
DownloadsTable
to verify it renders rows correctly for differentsource
inputs.
'use client';
Co-authored-by: Copilot <[email protected]> Signed-off-by: Caner Akdas <[email protected]>
apps/site/next.dynamic.constants.mjs
Outdated
@@ -29,6 +32,14 @@ export const IGNORED_ROUTES = [ | |||
* @type {Map<string, import('./types').Layouts>} A Map of pathname and Layout Name | |||
*/ | |||
export const DYNAMIC_ROUTES = new Map([ | |||
// Creates dynamic routes for downloads archive pages for each version | |||
// (e.g., /download/v18.20.8, /download/v20.19.2) | |||
...provideReleaseData() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My fear is that this will blow static builds in a monstruous way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also calling the provider right here in a constant file that could be called in a non-react environment might not be a good idea.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you have a number of how many more routes this generates?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently, there are 606 pages for the default locale alone. Should all of them be included in the static build, or should we selectively build only the ones that are most frequently accessed (lts, current, or last X version)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can do the following on the ignore list:
- locale isn't defaultLocale and paths starts with /download/archive
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder how long the static build takes in this branch?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Speed seems actually quite ok. Btw seeing this error on the build "Error: Cannot apply utility class after:md:-left-1/2
because the md
variant does not exist."
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Speed seems actually quite ok. Btw seeing this error on the build "Error: Cannot apply utility class
after:md:-left-1/2
because themd
variant does not exist."
Rebase, it's been fixed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Before: 1m 12s - 1587 static page
After: 1m 28s - 2172 static page
Of course, this is just a sample measurement; I’ll also keep track in the other builds we’ll be doing in this branch 🙌
status: NodeReleaseStatus; | ||
}; | ||
|
||
const WithReleaseAlertBox: FC<WithReleaseAlertBoxProps> = ({ status }) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice
@@ -70,8 +70,12 @@ const generateReleaseData = async () => { | |||
const status = getNodeReleaseStatus(new Date(), support); | |||
|
|||
const minorVersions = Object.entries(major.releases).map(([, release]) => ({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You know the more data you add, the bigger the whole thing becomes. I'm only afraid on bundle size changes and how bigger of a footprint this change especifically will do on our client-bundles and HTMLs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On pages that use this generator, I will perform a check. If necessary, I can separate the minor version information from this generator and create a different one, since standard download pages, as far as I recall, do not require this information 🤔
Lighthouse Results
|
|
||
<h2 className="flex items-center gap-2"> | ||
<img | ||
src="/static/images/favicons/favicon.png" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we have an SVG ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe Augustin is asking if we have an svg for the Node.js icon in @node-core/ui-components
. No, we do not have a Node.js icon-only component
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can use component but we don't have so :
- The less appealing option is not to include it.
- Use the same link as branding
- Add component for that
Description
This PR introduces a new statically generated page that displays simplified download options by major version:
Also includes a download table of other minor versions of the major version;

Since I included the minor versions as a table, the content became quite lengthy. To prevent this, I used the HTML
details
element. The main reason I didn't use Radix Primitive Accordion is that we want this page to be fully usable even for users who have JavaScript disabledI am open to suggestions for to add / remove content 🙇
Validation
The links below are accessible in the preview;
Related Issues
Addresses #7443
Check List
pnpm format
to ensure the code follows the style guide.pnpm test
to check if all tests are passing.pnpm build
to check if the website builds without errors.